Skip to content

Keep empty constant arrays out of the oversized merge in TypeCombinator::optimizeConstantArrays() - #6560

Merged
ondrejmirtes merged 2 commits into
phpstan:2.3.xfrom
phpstan-bot:create-pull-request/patch-sz9mpdv
Sep 23, 2026
Merged

ondrejmirtes merged 2 commits into
phpstan:2.3.xfrom
phpstan-bot:create-pull-request/patch-sz9mpdv

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

The bug-12671.php benchmark went from well under a second to ~26s. Bisecting pointed to e606ca2 ("Merge unions of constant arrays with many keys instead of degrading"). That commit exposed an existing weakness in TypeCombinator::optimizeConstantArrays(): a single array{} member kept the generalized oversized arrays from being merged into one. The union then compared hundreds of generalized arrays pairwise, each with a huge item-type union.

With this change, analysing the bench file takes ~1.7s (0.8s with the turbo extension) instead of ~18.5s locally.

Changes

  • src/Type/TypeCombinator.php (optimizeConstantArrays()): empty constant arrays are set aside before the oversized generalization. The remaining members are merged as before ($eachIsOversized is no longer spoiled by array{}), and the empty arrays are put back next to the result.
  • turbo-ext/src/TypeCombinator.cpp: same change in the native mirror. The regression test fails with the old native build and passes with the new one. Analysis output of the bench file is identical with and without the extension.
  • Probed analogous paths:
    • The general-array branch of processArrayTypes() goes through the same function, so it is covered.
    • The inner value traversal in the same method already skips empty arrays.
    • Non-constant array members still prevent the merge. That is intended, since their item types are not generalized.

Root cause

The oversized generalization turns every non-empty ConstantArrayType into non-empty-array<K, V>&oversized-array. When all members were generalized, it merges them into one array. Empty arrays are deliberately not generalized (array{}&oversized-array would be contradictory), but that also left $isOversized false for them, so $eachIsOversized became false and the method returned every generalized member separately.

In the benchmark, $pr_states[$country] over a country→states map (many countries map to []) produced ~250 such members. Their item types are unions of list{code, name, localName}&oversized-array. TypeCombinator::union() then ran compareTypesInUnion() over them pairwise, and every comparison was a union-vs-union isSuperTypeOf(). That made about 557k nested isSuperTypeOf calls for a single offset fetch.

Before e606ca2, a hidden 62-distinct-key cap degraded such unions to a general array before reaching this code, which masked the problem.

The result is also more consistent now: array{}|X is exactly array{} plus what the union gives without the empty member. Previously the pairwise path relied on the intentionally lossy "oversized intersection is a subtype when maybe" rule in IntersectionType::isSubTypeOf(). For example, it absorbed Argentina's states into Spain's array and dropped them from the inferred type.

Test

tests/PHPStan/Analyser/nsrt/bug-15293.php builds a constant map of three entries with disjoint keys (together over ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT values) plus one [] entry. It asserts that an unknown-key fetch yields array{}|(non-empty-array<…, list{…}&oversized-array>&oversized-array), identical to the map without the empty entry. Without the fix, the first assertion fails with a union of separate list{n, n, n}&oversized-array members. The existing tests/bench/data/bug-12671.php covers the performance side.

Fixes phpstan/phpstan#15293

🤖 Generated with Claude Code

phpstan-bot and others added 2 commits September 23, 2026 12:30
…tor::optimizeConstantArrays()`

- optimizeConstantArrays() generalizes every non-empty constant array of an
  oversized union to `non-empty-array<K, V>&oversized-array` and, when every
  member got generalized, merges them into a single oversized array. An
  `array{}` member is left alone by the generalization, so it set
  $eachIsOversized to false and the merge was skipped. The generalized
  members were then returned as separate union members.
- Since e606ca2 (unions of constant arrays with many keys no longer
  degrade early), fetching an unknown offset of a big constant map, where
  some entries are `[]` (bug-12671.php bench), hit this path. The union then
  compared hundreds of generalized arrays pairwise. Their item types are
  large unions of `list{...}&oversized-array`, so every comparison is itself
  a union-vs-union check, which made the fetch quadratic (bench 0.3s -> 26s).
- Set empty constant arrays aside before generalizing, merge the remaining
  members as before, and put the empty arrays back next to the result. A
  union with an `array{}` member now gives `array{}` plus exactly what the
  same union without it gives.
- Ported the same change to the native mirror in
  turbo-ext/src/TypeCombinator.cpp.
- Probed the other paths that feed optimizeConstantArrays() (the
  general-array branch of processArrayTypes() reuses the same function, so it
  is covered). The inner value traversal already left empty arrays alone.
- Non-constant array members also prevent the merge. That is intended
  (their item types are not generalized), so it is unchanged.
@ondrejmirtes
ondrejmirtes force-pushed the create-pull-request/patch-sz9mpdv branch from 5827960 to b940151 Compare September 23, 2026 10:30
@ondrejmirtes
ondrejmirtes merged commit b940151 into phpstan:2.3.x Sep 23, 2026
209 of 212 checks passed
@ondrejmirtes
ondrejmirtes deleted the create-pull-request/patch-sz9mpdv branch September 23, 2026 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance bottleneck in bug-12671.php

2 participants